Skip to content

fix(auth): check if user disabled on check_revoked #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 4, 2021

Conversation

bojeil-google
Copy link
Contributor

When verify_session_cookie or verify_id_token is called with
check_revoked set to True we should also check if the user is
disabled.

If disabled the UserDisabledError is raised.

When `verify_session_cookie` or `verify_id_token` is called with
`check_revoked` set to `True` we should also check if the user is
disabled.

If disabled the `UserDisabledError` is raised.
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM!

@bojeil-google
Copy link
Contributor Author

Thanks! LGTM!

Thanks for the quick review. we are planning to fix this in Node.js, Java and Golang too.

@lahirumaramba
Copy link
Member

Added @egilmorez for docs changes. Thanks!

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good. Just had one suggestion about the base type for the new UserDisabledError class.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a suggestion.

Copy link

@egilmorez egilmorez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc strings look good, thanks!

@bojeil-google bojeil-google removed their assignment Aug 4, 2021
@bojeil-google bojeil-google merged commit 0e35c9a into master Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants